This PR needs a basic regression test that ensures Pubsub.SeenMessagesTTL set by the user is respected.
Unsure what is the best way to test this, you'd need to set it too low and then observe duplicates?

@smrz2001 we have some prior art in test/sharness/t0320-pubsub.sh if that helps.

Hey @lidel, it took me a bit to get everything figured out and setup with sharness. After playing around with it a bit, I'm realizing that there isn't really a good way to induce duplicate pubsub messages. The API (rightly) doesn't allow setting a message ID explicitly.

func (t *Topic) Publish(ctx context.Context, data []byte, opts ...PubOpt) error {
.
.
.
	return t.p.val.PushLocal(&Message{m, "", t.p.host.ID(), nil, pub.local})
}

The only option I can currently come up with is to use tc netem via command line or code to simulate network traffic duplication.

Does this seems like a viable approach? Not sure what the restrictions are around what tests are/aren't allowed to do. I've used tc netem before so I can figure this out, if acceptable.

Thank you for looking into this. I hoped there is an easier way, but I guess we've hit an edge case on this one.

We already have some helpers for sharness in ./test/dependencies, such as pollEndpoint, so that is a potential path forward (golang helper is a bit more future-proof, could be reused when we move away from the sharness). Using tc netem is also an option, if we can get it to work on CI.

@marten-seemann @vyzo is there a better / less painful way to test WithSeenMessagesTTL / simulate duplicated messages?

I'm thinking aloud here but I could also make a libp2p-sim tool under test/dependencies that connects to libp2p like a normal node but is capable of sending spurious messages. Might be useful in other contexts too perhaps.

probably not, we do go out of our way to supress them.

What do you think about having a libp2p-sim that implements it's own Publish and can send dups?

This and libp2p/go-libp2p-pubsub#502 both sound sensible 👍

@lidel, would it be possible to do a regular Golang unit test here instead of adding to sharness?

@lidel, after a lot of hair pulling, I was unable to set up a libp2p simulator but I was able to figure out how to write a Go test under test/integration that does the job, IMO. Would it work to keep the test there? Let me know what you think.

FYI, this test uncovered another pubsub/cache bug, which I will fix under this issue.

Thank you for doing due diligence on this, yes, your approach is acceptable: this is the best we can do given tools at hand, and it still acts as an early warning if go-libp2p APIs change, or a regression is introduced.

Thank you for driving this @smrz2001, and being patient with test work. I'm merging this, so we can include it in Kubo 0.18.0-rc1 (ETA second week of December, or beginning of January).

Excellent, thanks so much, @lidel!! 🙏🏼🙏🏼

I'll try to get the other bugs fixed soon as well, and will update the dependencies and tests accordingly.

Thank you for doing due diligence on this, yes, your approach is acceptable: this is the best we can do given tools at hand, and it still acts as an early warning if go-libp2p APIs change, or a regression is introduced.